Skip to content

fix(windows): patch platform._syscmd_ver to survive non-UTF-8 output under PEP 540 - #69522

Closed
webtecnica wants to merge 2 commits into
NousResearch:mainfrom
webtecnica:fix/69413-windows-utf8
Closed

fix(windows): patch platform._syscmd_ver to survive non-UTF-8 output under PEP 540#69522
webtecnica wants to merge 2 commits into
NousResearch:mainfrom
webtecnica:fix/69413-windows-utf8

Conversation

@webtecnica

@webtecnica webtecnica commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Fixes #69413

Description

On Windows, the OpenAI SDK generates platform headers by calling platform.platform(). This triggers:

platform.platform()
→ platform.win32_ver()
→ platform._syscmd_ver()
→ subprocess.check_output("ver", text=True, encoding="locale", shell=True)
→ subprocess.Popen._readerthread

When PYTHONUTF8=1 (PEP 540 UTF-8 mode) is active — which Hermes sets on Windows — locale.getencoding() returns "utf-8". However, the Windows ver command outputs text in the system's active ANSI code page (e.g. cp1252 on US-English). Bytes that are valid in cp1252 but not in UTF-8 — such as 0xe9 (é in cp1252, an invalid UTF-8 start byte) — raise UnicodeDecodeError inside the subprocess reader thread.

Fix

Added _patch_platform_syscmd_ver() to hermes_bootstrap.py that wraps platform._syscmd_ver with a try/except UnicodeDecodeError on Windows. When the ver command output can't be decoded as UTF-8, the patched function gracefully returns its input defaults.

Applied automatically on module import (every Hermes entry point already imports hermes_bootstrap first). No-op on POSIX.

Changes

File Change
hermes_bootstrap.py Added _patch_platform_syscmd_ver() function + module-level call
tests/test_hermes_bootstrap.py Added TestPatchPlatformSyscmdVer class (5 tests)

Test results

All 23 tests pass, 5 Windows-only skipped on Linux.

Closes #69413

@webtecnica
fix(agent): prevent auto retry loop from slow LLM backend prefilling …
47bad20 
…large contexts (NousResearch#69424)

Three-pronged fix for the stale-stream detector killing connections
before a slow local/cloud model finishes prompt prefill:

1. Apply context-size scaling to local endpoints too
   The local-endpoint stale-timeout branch (default 900s) skipped the
   context-token scaling that the cloud path applied, so a 900s flat
   ceiling could still fire before a 122B model finishes prefilling
   140K+ tokens.  Move scaling out of the  branch so both local
   and cloud paths get proportional timeouts:
   - >200K tokens → 1800s (30 min)
   - >100K tokens → 1200s (20 min)
   - >50K  tokens → 600s  (10 min)

2. Add stale-streak backoff
   After 2+ consecutive stale kills, apply a progressive multiplier
   (1× → 2.5× → 4× … up to 10×) to the stale timeout so each retry
   waits longer, eventually outlasting the prefill and breaking the
   infinite retry loop.  Resets on successful response.

3. Raise the non-streaming stale timeout tiers consistently
   The non-streaming path () and
   Bedrock path () now share the same
   increased floors for consistency.

Closes NousResearch#69424.
@webtecnica
fix(windows): patch platform._syscmd_ver to survive non-UTF-8 output …
fde3e14 
…under PEP 540

On Windows with PYTHONUTF8=1 (PEP 540 UTF-8 mode),
platform._syscmd_ver() calls subprocess.check_output(...,
text=True, encoding='locale', shell=True) to run the Windows
'ver' command.  Under PEP 540, locale.getencoding() returns
'utf-8', but the 'ver' command emits output in the system's
active ANSI code page (cp1252 on US-English).  Bytes that are
valid in cp1252 but not in UTF-8 — e.g. 0xe9 (é) — raise a
UnicodeDecodeError inside the subprocess reader thread.

The patch wraps platform._syscmd_ver with a try/except that
catches UnicodeDecodeError and gracefully returns input defaults,
matching the existing OSError/CalledProcessError fallback that
_win32_ver already handles.

Fixes NousResearch#69413
@isak-ialogics

isak-ialogics commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Triage evidence only (not a merge decision): this PR currently contains commit 47bad20, which is the complete head commit of the separate #69521. As a result, the diff here includes unrelated changes to agent/chat_completion_helpers.py, run_agent.py, and tests/agent/test_non_stream_stale_timeout.py in addition to the Windows fix described by this PR. Concrete next action: rebase this branch onto current main and retain only fde3e142 (or otherwise drop 47bad20), then rerun CI so #69522 is scoped to hermes_bootstrap.py and its regression tests.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows labels Jul 22, 2026
@teknium1 teknium1 mentioned this pull request Jul 24, 2026
Merged
@teknium1

teknium1 commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Closing after a deep verification pass — the diagnosis deserves a detailed response because the issue you chased is partially real.

What's correct: on Python 3.11.0/3.11.1, platform._syscmd_ver lacks CPython's encoding="locale" fix (added in 3.11.2), so under PEP 540 UTF-8 mode the cmd /c ver output is strict-utf-8 decoded and raises on OEM-code-page bytes. That crash path was live in processes that don't run hermes_cli.main (slash workers, tui_gateway entry, run_agent).

Why this PR couldn't merge as-is: (1) the wrapper's supported_platforms=None default is forwarded into the original, which does sys.platform not in supported_platforms → guaranteed TypeError on every no-arg call — platform.win32_ver() invokes it exactly that way, so on Windows the patch breaks platform.platform() outright (the tests mock past this); (2) for Python ≥3.11.2, locale.getencoding() explicitly ignores UTF-8 mode, so the PR's core claim doesn't hold there; (3) the branch bundles an unrelated commit (#69521's stream-timeout changes).

What shipped instead (PR #71014): the existing full-stub suppress_platform_ver_console() now runs from hermes_bootstrap, covering every entry point — win32_ver() falls back to sys.getwindowsversion(), fixing both the decode crash on the old micros and the console flash, with a regression test pinning the no-arg-call contract. Thanks for surfacing the coverage gap — the bootstrap move exists because your PR pointed at the right set of exposed processes.

@teknium1 teknium1 closed this Jul 24, 2026
@webtecnica webtecnica mentioned this pull request Aug 25, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Windows subprocess reader thread crashes on non-UTF-8 output

4 participants

@webtecnica @isak-ialogics @teknium1 @alt-glitch